Skip to content

ebpf: simplify get_pristine_per_cpu_record#1091

Merged
fabled merged 1 commit into
open-telemetry:mainfrom
fabled:tt-simplify-pristine-percpy
Jan 20, 2026
Merged

ebpf: simplify get_pristine_per_cpu_record#1091
fabled merged 1 commit into
open-telemetry:mainfrom
fabled:tt-simplify-pristine-percpy

Conversation

@fabled
Copy link
Copy Markdown
Contributor

@fabled fabled commented Jan 19, 2026

No description provided.

@fabled fabled requested review from a team as code owners January 19, 2026 14:04
@fabled fabled force-pushed the tt-simplify-pristine-percpy branch from ab5e0d0 to ba24057 Compare January 19, 2026 14:20
@fabled fabled merged commit 40dd9d9 into open-telemetry:main Jan 20, 2026
29 checks passed
gnurizen added a commit to parca-dev/opentelemetry-ebpf-profiler that referenced this pull request May 22, 2026
This regressed in 40dd9d9 ("ebpf: simplify
get_pristine_per_cpu_record" open-telemetry#1091), which removed an explicit zeroing of
trace->custom_labels.labels on the assumption that the slots were only
read after a successful write that fully populated them. The slots are
populated via bpf_probe_read_user, which only writes the bytes it reads,
so a key/value shorter than one previously written to the same per-CPU
slot inherits the trailing bytes from the prior trace and produces a
corrupted label.

The eBPF side now writes a single NUL after each bpf_probe_read_user;
userspace stops at the first NUL, so one terminator is sufficient. On
the Go side, label keys/values are validated as UTF-8 (required by
OTLP/pprof). On fixed-width truncation that splits a multi-byte rune we
salvage the longest valid UTF-8 prefix rather than drop the whole label.
Empty keys and unsalvageable values are dropped, and two new metrics
expose how often each case occurs.

Comm is left as best-effort: it's kernel-supplied, almost always ASCII,
and we don't have a useful fallback if it isn't.
gnurizen added a commit to parca-dev/opentelemetry-ebpf-profiler that referenced this pull request May 22, 2026
This regressed in 40dd9d9 ("ebpf: simplify
get_pristine_per_cpu_record" open-telemetry#1091), which removed an explicit zeroing of
trace->custom_labels.labels on the assumption that the slots were only
read after a successful write that fully populated them. The slots are
populated via bpf_probe_read_user, which only writes the bytes it reads,
so a key/value shorter than one previously written to the same per-CPU
slot inherits the trailing bytes from the prior trace and produces a
corrupted label.

The eBPF side now writes a single NUL after each bpf_probe_read_user;
userspace stops at the first NUL, so one terminator is sufficient. On
the Go side, label keys/values are validated as UTF-8 (required by
OTLP/pprof). On fixed-width truncation that splits a multi-byte rune we
salvage the longest valid UTF-8 prefix rather than drop the whole label.
Empty keys and unsalvageable values are dropped, and two new metrics
expose how often each case occurs.

Comm is left as best-effort: it's kernel-supplied, almost always ASCII,
and we don't have a useful fallback if it isn't.
gnurizen added a commit to parca-dev/opentelemetry-ebpf-profiler that referenced this pull request May 22, 2026
This regressed in 40dd9d9 ("ebpf: simplify
get_pristine_per_cpu_record" open-telemetry#1091), which removed an explicit zeroing of
trace->custom_labels.labels on the assumption that the slots were only
read after a successful write that fully populated them. The slots are
populated via bpf_probe_read_user, which only writes the bytes it reads,
so a key/value shorter than one previously written to the same per-CPU
slot inherits the trailing bytes from the prior trace and produces a
corrupted label.

The eBPF side now writes a single NUL after each bpf_probe_read_user;
userspace stops at the first NUL, so one terminator is sufficient. On
the Go side, label keys and values are validated as UTF-8 (required by
OTLP/pprof), with deliberately asymmetric strictness:

  - Keys are strict: any invalid byte (including a single split-rune
    continuation at the end) drops the whole label. A corrupted key
    would silently group unrelated samples under a garbage name.
  - Values are lenient: on fixed-width truncation that splits a
    multi-byte rune we salvage the longest valid UTF-8 prefix rather
    than drop the label, so a clipped request_id or customer_name still
    arrives with everything up to the broken rune intact.

Two metrics count labels dropped for each reason. Comm is left as
best-effort: it's kernel-supplied, almost always ASCII, and we don't
have a useful fallback if it isn't.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants